Skip to content

PCT: Change the Connect default to use OHE#672

Merged
bschwedler merged 10 commits intomainfrom
tn-change-connect-default
Jun 9, 2025
Merged

PCT: Change the Connect default to use OHE#672
bschwedler merged 10 commits intomainfrom
tn-change-connect-default

Conversation

@tnederlof
Copy link
Contributor

This PR changes the Connect chart to run in OHE mode by default instead of the current default (service and content runs in the same pod(s)). I think this default makes more sense as its the way we at Posit advise customers to run Connect in Kubernetes and it is the only Kubernetes configuration documented in the admin guide.

This is also important for customers who have Pod Security Standards or other mechanisms set to disallow privileged execution. The current chart runs with securityContext.privileged: true, which causes issues for these customers until they know to switch this setting. This PR will eliminate that need.

Bumped the minor version and added a section to the README template to make it more prominent the potentially breaking change (for customers who are not running in OHE).

Once this merges, the documentation here can be simplified (don't need to explicitly have folks add launcher.enabled: true. https://docs.posit.co/connect/admin/getting-started/off-host-install/configure-helm-chart/

Closes: #436

@tnederlof
Copy link
Contributor Author

It looks like the install in CI is failing because there is no PVC. I am not familiar with the testing infrastructure. Is there a way to mock up storage in these tests? It seems this error is reasonable if a customer is trying to setup without sharedStorage.

Error: template: rstudio-connect/templates/NOTES.txt:46:6: executing "rstudio-connect/templates/NOTES.txt" at <fail "\n\nWhen launcher is enabled, persistent storage must be provided.\nThis is usually done via a PersistentVolumeClaim (PVC) with `sharedStorage.create=true`, although there are other options.">: error calling fail: HELM_ERR_START

When launcher is enabled, persistent storage must be provided.
This is usually done via a PersistentVolumeClaim (PVC) with `sharedStorage.create=true`, although there are other options.HELM_ERR_END

@bdeitte
Copy link
Member

bdeitte commented May 17, 2025

@jforest In the comment above, is this the part of the testing infra you worked on?

@jforest
Copy link
Contributor

jforest commented May 19, 2025

@jforest In the comment above, is this the part of the testing infra you worked on?

This is the install testing, yes. This is triggered because the chart requires a pvc when launcher.enabled is true

https://github.com/rstudio/helm/blob/main/ci/rstudio-connect/install/license-file-values.yaml is the only set of install testing configs. Each file in that directory will be used for a different install test. These tests are run by the chart testing tool. The lint tests are also run by that tool.

Would you like to set up a time to talk through how all the testing is set up?

@tnederlof
Copy link
Contributor Author

@jforest gave me a walkthrough of the testing and was able to adjust the CI run so it creates a PVC so tests pass now. This should be ready for review.

* main: (59 commits)
  By, by, by
  Update helm-docs and README.md
  Add product docs link
  Add product docs link
  Update helm-docs and README.md
  Reword Workbench API key section
  Trigger CI
  Update helm-docs and README.md
  Improvements and fixes to the README agent sections
  Update helm-docs and README.md
  Improvements and fixes to values and documentation
  Trigger CI
  Update helm-docs and README.md
  Add `chronicleAgent.agentEnvironment` value and set `CHRONICLE_AGENT_ENVIRONMENT` if it is defined
  Trigger CI
  Update helm-docs and README.md
  Add securityContext for Chronicle agent container with expectation of non-root and unprivileged execution
  Trigger CI
  Trigger CI
  Change `chart` to `release` when refering to existing install
  ...
@bschwedler bschwedler force-pushed the tn-change-connect-default branch from baa9969 to 2677ba2 Compare June 9, 2025 16:29
@bschwedler bschwedler requested review from a team as code owners June 9, 2025 16:29
@bschwedler bschwedler force-pushed the tn-change-connect-default branch 2 times, most recently from 60b623f to fd9a2c0 Compare June 9, 2025 16:35
* main:
  Bump Workbench to 2025.05.1
  Use latest chart-releaser action and cr.yaml config
  Bump Package Manager version to 2025.04.2
  NEWS and versions
  remove CA enrollment
  simpler certification copying; remove unused variables
  Specify major version for Actions
  Update helm/kind-action to latest
  Add .helmignore to .helmignore so chart-testing ignores changes
  Use helmignore for chart-testing and unittest
  Move test files into chart directories
  Update .helmignore to skip test directories.
  Add launcher project to what can be tagged
  trigger ci
  Update helm-docs and README.md
  Bump Connect to version 2025.05.0
@bschwedler bschwedler force-pushed the tn-change-connect-default branch from fd9a2c0 to dcf9a1b Compare June 9, 2025 16:36
@bschwedler
Copy link
Contributor

@aronatkins @dbkegley @zackverham
Do you forsee any issues merging this PR?

@dbkegley
Copy link
Contributor

dbkegley commented Jun 9, 2025

I don't see any issues besides the obvious one where customers will accidentally enable off-host after this upgrade. That said, I'm happy to see us finally changing the chart default.

@bschwedler bschwedler merged commit 6236c55 into main Jun 9, 2025
7 checks passed
@bschwedler bschwedler deleted the tn-change-connect-default branch June 9, 2025 17:21
@tnederlof
Copy link
Contributor Author

I don't see any issues besides the obvious one where customers will accidentally enable off-host after this upgrade. That said, I'm happy to see us finally changing the chart default.

Yes, hopefully it doesn't happen to often but I will give support a heads up. Glad to see it happen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change launcher for Connect chart to default true

6 participants